Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: debt limit changes no longer break partial liquidations #250

Merged
merged 5 commits into from
Sep 1, 2024

Conversation

lekhovitsky
Copy link
Collaborator

@StErMi
Copy link

StErMi commented Aug 25, 2024

The implementation is fine. Having a custom behavior when the repayment happens via the liquidation problems would create more harm than benefits because it could be exploited by self-partial-liquidations.

@StErMi
Copy link

StErMi commented Aug 28, 2024

@lekhovitsky @cryptarasecurity I'll be more specific to describe the changes made and the effects

The main differences are inside the CreditFacadeV3 internal function _revertIfOutOfDebtLimits

BEFORE: reverts if the new debt of the CA was below minDebt or above maxDebt
AFTER:

  • reverts if the action == INCREASE_DEBT and debt < minDebt || debt > maxDebt
  • reverts if the action == DECREASE_DEBT and debt < minDebt and debt > 0

What does it change?

  1. For borrowing: nothing. When you borrow (increase debt) your debt must be within the limits.
  2. For repaying / partial liquidating (full repay, or full liquidation will early return): you will be able to decrease the debt up to minDebt without worrying about the maxDebt upper bound (unlike before). This means that even if the DAO reduces the maxDebt too much and the CA debt was already above the upper threshold, you will still be able to repay/liquidate the debt even for just 1 wei without reverting the transaction.

Some of the concerns/side effects detailed in the issue https://github.com/spearbit-audits/review-gearbox/issues/60 are still there but the main one, at least in my opinion have been mitigated correctly: users that need to repay/liquidate the CA will be able to do so even if the DAO has reduced the maxDebt upper bound "too much" (relative to their debt). Such operation, when the final debt is above the min threshold (or fully remove the debt) should always be doable.

Given that the minDebt and maxDebt thresholds are there for a reason, we need to arrive at a point where the DAO must be trusted to choose valuable, meaningful and trusted values for those thresholds:

  1. Reducing maxDebt too much will prevent users from increasing their borrow position or creating new borrow positions at all
  2. Reducing minDebt too much will prevent users from creating new borrow positions or reducing their debt (via repay/partial liquidation)

Both scenarios are critical and for that reason, the Gearbox DAO must choose meaningful values for both minDebt and maxDebt

Base automatically changed from spearbit-11 to spearbit-fixes September 1, 2024 10:09
@lekhovitsky lekhovitsky merged commit 693de97 into spearbit-fixes Sep 1, 2024
3 checks passed
@lekhovitsky lekhovitsky deleted the spearbit-60 branch September 1, 2024 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants